fix: inert react 19 support#12315
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (8)
WalkthroughReplaced empty-string ChangesIS_INERT migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview: https://pf-react-pr-12315.surge.sh A11y report: https://pf-react-pr-12315-a11y.surge.sh |
logonoff
force-pushed
the
12295-drawerpanel
branch
from
a88edd6 to
3c37bfc
Compare
logonoff
changed the title
|
@coderabbitai review Check if it's possible and compatible with React 17, 18, and 19 to just use |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 16858 🏁 Script executed: Length of output: 278 🌐 Web query: 💡 Result: In React 19, What happens with
|
| React version | inert: 'true' behavior |
Result |
|---|---|---|
| 17 / 18 | inert is unrecognized; React passes the string through to the DOM as inert="true" |
✅ Functionally works (browser treats any value as present/truthy for boolean attributes) |
| 19 | inert is now a first-class boolean attribute |
❌ React 19 emits a dev warning: "Received the string true for the boolean attribute inert… Did you mean inert={true}?" |
In React 19, inert is treated as a real boolean DOM attribute. Passing the string "true" triggers a React warning about receiving a string for a boolean attribute, similar to how disabled="true" behaves. This is essentially the same class of warning the PR is trying to resolve, just with a different value.
The current implementation in helpers/inert.ts is the correct approach:
// React < 19: empty string '' → attribute presence in DOM → element is inert ✓
// React >= 19: boolean true → correct native boolean value → no warning ✓
export const IS_INERT = Number(version.split('.')[0]) >= 19 ? true : ('' as unknown as boolean);- In React < 19,
inert=""(empty string) was passed through to the DOM as attribute presence, making the element inert. The'' as unknown as booleancast satisfies TypeScript while preserving that behavior. - In React 19,
inert=""is treated asfalse, breaking the old workaround, so the version check correctly switches totrue(boolean).
There's no single value that works warning-free across all three React major versions — the version-guarded IS_INERT helper is the right solution.
[review]
🧠 Learnings used
Learnt from: arpanroy41
Repo: patternfly/patternfly-react PR: 12237
File: packages/react-core/src/components/Toolbar/examples/ToolbarFlexGrowAndWidths.tsx:6-6
Timestamp: 2026-02-06T19:58:56.464Z
Learning: In the patternfly-react repository, default imports from 'react' (e.g., `import React from 'react'`) are restricted. React is available globally through the build configuration, so `React.FunctionComponent` and similar React APIs can be used without importing React.
Learnt from: tlabaj
Repo: patternfly/patternfly-react PR: 12288
File: packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx:144-150
Timestamp: 2026-03-24T14:47:38.734Z
Learning: In PatternFly React, `isPlain` is a public component prop that defaults to `false` by convention, consistent with other PatternFly components. Omitting `isPlain` is treated as equivalent to `isPlain={false}`. Do not suggest changing the default to `undefined` or altering `!isPlain`-based conditional logic, as this would diverge from established PatternFly API patterns and could be a behavioral breaking change.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-table/src/components/Table/Tr.tsx`:
- Line 7: The Tr.tsx file imports IS_INERT from a private path; replace that
private import by either (A) importing IS_INERT from react-core's public
entrypoint (e.g., from '@patternfly/react-core' or the public helpers index if
the project exports it) or (B) remove the private dependency and implement the
inert detection inline in the Tr component (define a local IS_INERT constant
using the same feature-detection logic used in react-core), then use that local
symbol wherever IS_INERT is referenced in Tr.tsx.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5dde64ab-c303-412d-a62d-d54fd01ce0ff
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
packages/react-core/src/components/Compass/Compass.tsxpackages/react-core/src/components/Drawer/DrawerPanelContent.tsxpackages/react-core/src/components/DualListSelector/DualListSelectorTreeItem.tsxpackages/react-core/src/components/Form/InternalFormFieldGroup.tsxpackages/react-core/src/components/Nav/NavExpandable.tsxpackages/react-core/src/components/SearchInput/SearchInput.tsxpackages/react-core/src/components/TreeView/TreeViewListItem.tsxpackages/react-core/src/helpers/inert.tspackages/react-table/src/components/Table/Tr.tsx
logonoff
force-pushed
the
12295-drawerpanel
branch
from
3c37bfc to
71ccccd
Compare
|
@kmcfaul / @nicolethoen please take a look, thanks! |
logonoff
force-pushed
the
12295-drawerpanel
branch
from
71ccccd to
adcb841
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/react-core/src/helpers/inert.ts (1)
3-7: Tighten the JSDoc — the empty string isn't treated as truthy by React JSX.The mechanism is slightly different from what the comment implies. In React 17/18,
inertis not a recognized DOM attribute, so React forwards whatever you pass (including"") directly to the DOM, and the DOM treats the presence of the attribute as "inert" regardless of value. React 19 addedinertto its known boolean‑attribute list, so an empty string is now coerced tofalse(attribute removed) — hence the need to pass booleantrue. Recommend updating the doc and adding a short note that thebooleantype is a deliberate widening so consumers can spread it as a JSX prop without ceremony.📝 Suggested doc tweak
/** - * Before React 19, React JSX treats empty string "" as truthy for inert prop. + * Cross-version value for the `inert` DOM attribute. + * + * - React >= 19 recognizes `inert` as a boolean attribute and requires `true` + * (an empty string would be coerced to `false` and the attribute removed). + * - React < 19 does not recognize `inert`, so an empty string is forwarded + * to the DOM as `inert=""`; the DOM then treats the attribute's presence + * as inert regardless of value. + * + * The runtime value is widened to `boolean` so consumers can spread it as a + * JSX prop without dealing with a `boolean | ''` union. * `@see` {`@link` https://stackoverflow.com/questions/72720469} */ export const IS_INERT = Number(version.split('.')[0]) >= 19 ? true : ('' as unknown as boolean);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-core/src/helpers/inert.ts` around lines 3 - 7, The JSDoc for IS_INERT is misleading about how empty string is treated; update the comment above the exported constant IS_INERT to explain that in React 17/18 `inert` is unknown so React forwards attributes verbatim and the DOM treats presence of the attribute as truthy regardless of value, whereas React 19 treats `inert` as a known boolean attribute so empty string is coerced to false (removed), and also add a short note that the type widening to boolean (the cast) is deliberate so consumers can spread IS_INERT as a JSX prop without needing to convert types; reference the IS_INERT constant and the version variable in the comment so future readers know why the runtime check exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-core/src/helpers/inert.ts`:
- Around line 1-7: The current IS_INERT computation reads React's version string
naively and will mis-handle prerelease tags like "0.0.0-experimental-...";
update the export const IS_INERT logic to robustly extract the numeric major
version from the version string (e.g. use a regex to find the first contiguous
digits /(\d+)/, parseInt that result, and fall back to 0 when no numeric major
is found) and then compute Number(major) >= 19; change the symbol export const
IS_INERT accordingly so prerelease formats safely fall back to the conservative
behavior.
---
Nitpick comments:
In `@packages/react-core/src/helpers/inert.ts`:
- Around line 3-7: The JSDoc for IS_INERT is misleading about how empty string
is treated; update the comment above the exported constant IS_INERT to explain
that in React 17/18 `inert` is unknown so React forwards attributes verbatim and
the DOM treats presence of the attribute as truthy regardless of value, whereas
React 19 treats `inert` as a known boolean attribute so empty string is coerced
to false (removed), and also add a short note that the type widening to boolean
(the cast) is deliberate so consumers can spread IS_INERT as a JSX prop without
needing to convert types; reference the IS_INERT constant and the version
variable in the comment so future readers know why the runtime check exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 755418a0-9e4c-4f6c-8a98-09dd2604fa26
📒 Files selected for processing (10)
packages/react-core/src/components/Compass/Compass.tsxpackages/react-core/src/components/Drawer/DrawerPanelContent.tsxpackages/react-core/src/components/DualListSelector/DualListSelectorTreeItem.tsxpackages/react-core/src/components/Form/InternalFormFieldGroup.tsxpackages/react-core/src/components/Nav/NavExpandable.tsxpackages/react-core/src/components/SearchInput/SearchInput.tsxpackages/react-core/src/components/TreeView/TreeViewListItem.tsxpackages/react-core/src/helpers/inert.tspackages/react-table/src/components/Table/Tr.tsxpackages/react-table/src/components/Table/utils/inert.ts
✅ Files skipped from review due to trivial changes (4)
- packages/react-core/src/components/Drawer/DrawerPanelContent.tsx
- packages/react-core/src/components/TreeView/TreeViewListItem.tsx
- packages/react-core/src/components/Nav/NavExpandable.tsx
- packages/react-core/src/components/Form/InternalFormFieldGroup.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react-table/src/components/Table/utils/inert.ts
- packages/react-core/src/components/SearchInput/SearchInput.tsx
| import { version } from 'react'; | ||
|
|
||
| /** | ||
| * Before React 19, React JSX treats empty string "" as truthy for inert prop. | ||
| * @see {@link https://stackoverflow.com/questions/72720469} | ||
| */ | ||
| export const IS_INERT = Number(version.split('.')[0]) >= 19 ? true : ('' as unknown as boolean); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is versiona stable named export from thereact package across React 17, 18, and 19, and what format does it follow?
💡 Result:
Yes. version is a stable named export from the react package across React 17, 18, and 19. Format: it follows the same “version string” format React uses for stable vs prerelease npm channels. For the Latest (stable) channel it’s SemVer-style major.minor.patch (e.g. 18.0.0), but for prerelease channel builds (Canary/Experimental) React generates a version string that includes extra metadata (hash + commit date), in the pattern shown by React’s versioning policy docs (e.g. 18.3.0-canary-- / 0.0.0-experimental--).
Citations:
- 1: https://stackoverflow.com/questions/36994564/how-can-one-tell-the-version-of-react-running-at-runtime-in-the-browser/36994601
- 2: NPM "version" export format changed (now includes SHA and date) facebook/react#24215
- 3: https://beta.reactjs.org/community/versioning-policy
- 4: https://react.dev/community/versioning-policy
- 5: https://react.dev/reference/react-compiler/target
React's version export is stable, but prerelease builds require special handling.
The version export is confirmed as a stable named export across React 17, 18, and 19. For stable releases, it follows SemVer format (major.minor.patch), so version.split('.')[0] reliably extracts the major version. However, prerelease builds (Canary/Experimental) use an extended format like 0.0.0-experimental-<hash>-<YYYYMMDD>, which would cause the version parsing logic to extract 0 instead of the actual major version. Consider adding a safeguard if prerelease versions need to be handled explicitly, or document that this code path uses the conservative fallback for prerelease builds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-core/src/helpers/inert.ts` around lines 1 - 7, The current
IS_INERT computation reads React's version string naively and will mis-handle
prerelease tags like "0.0.0-experimental-..."; update the export const IS_INERT
logic to robustly extract the numeric major version from the version string
(e.g. use a regex to find the first contiguous digits /(\d+)/, parseInt that
result, and fall back to 0 when no numeric major is found) and then compute
Number(major) >= 19; change the symbol export const IS_INERT accordingly so
prerelease formats safely fall back to the conservative behavior.
logonoff
force-pushed
the
12295-drawerpanel
branch
from
adcb841 to
d0060bb
Compare
2 participants
What: Closes #12295
It might be possible that just using
inert: 'true'also fixes this problem but I have not verified if that worksAdditional issues: issue due to https://stackoverflow.com/questions/72720469/error-when-using-inert-attribute-with-typescript
Summary by CodeRabbit